fix(cli): lint sets process.exitCode instead of process.exit() (flush stdout) [P1]#1936
Conversation
… stdout) `hyperframes lint --json` wrote the JSON payload with console.log() and then immediately called process.exit(). process.exit() terminates the process before Node flushes an asynchronously-buffered stdout, which is what a non-TTY (piped) stdout is — so `hyperframes lint --json | tee`, `> out.json`, or any agent/CI capture silently lost the entire payload on Windows (reported on 0.7.31 non-TTY). The same console.log-then-exit pattern was on all four exit sites (both --json branches and the human-readable + thrown-error paths), so any of them could truncate. Fix: set process.exitCode and return, letting run() unwind so Node drains stdout before exiting with the code. This is exactly the pattern the other commands (publish/transcribe/upgrade/play/present) already use; lint was the outlier still calling process.exit() after writing. Test: new lint.test.ts drives the command's run() with mocked lintProject/resolveProject and a process.exit spy that throws if called. Covers the --json-with-errors, --json-clean, --json-thrown, and human-readable paths — each asserts process.exit is never called and the correct exitCode is set. Fails against the pre-fix code (the spy throws on the first process.exit).
miguel-heygen
changed the title
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Review: LGTM ✅
Clean, focused fix for a real P1 data-loss bug. The process.exitCode + return pattern is idiomatic Node and matches how the other HyperFrames CLI commands already handle exits.
Verified
- Root cause correct:
process.exit()terminates before Node flushes asynchronously-buffered stdout on non-TTY (piped) output. Replacing withprocess.exitCode = N; return;lets the event loop drain naturally. - All four exit paths covered:
--jsonwith errors,--jsonclean,--jsonon thrown error, and human-readable with errors — all switched fromprocess.exit()toprocess.exitCode. - No hang risk: The lint command doesn't open persistent handles (timers, connections), so the process exits naturally after the event loop drains.
- Tests well-designed: The
process.exitspy that throws if called is a smart regression guard — any reintroduction ofprocess.exit()immediately fails the test.
Suggestions (out of scope, non-blocking)
-
validatehas the same issue —validate.ts:615,619uses the identicalconsole.log(JSON.stringify(...)); process.exit(exitCode)pattern with--json. Worth a follow-up to apply the same fix there. -
resolveProject()also callsprocess.exit(1)—project.ts:65onInvalidProjectError. If lint is invoked with a bad directory and--json, stdout truncation can still happen beforelint.run()starts. Separate concern. -
Test gap: Human-readable clean path (0 errors, 0 warnings) isn't tested. That path does a plain
return(unchanged by this PR), so no regression risk — just noting for completeness.
Ship it.
— Miga
miguel-heygen
changed the title
|
@miga thanks for the review. On the out-of-scope note: you're right that |
aszala-hg
left a comment
aszala-hg
left a comment
There was a problem hiding this comment.
Verified independently: all four exit sites in packages/cli/src/commands/lint.ts are converted to process.exitCode + return, with no process.exit() remaining anywhere in the file at ee5a90c, and no fall-through past any exit path. The new lint.test.ts genuinely pins the regression — a process.exit spy that throws if called, plus exitCode assertions across all four paths. No hang risk: lintProject is pure fs + linkedom parsing (no servers/watchers/timers) and withMeta is a synchronous cache read. All required checks green at head ee5a90c (the earlier "Tests on windows-latest" failure was an unrelated engine-package videoFrameExtractor afterAll rmSync flake — same workflow had passed at this exact SHA; re-run passed).
One line on Miga's out-of-scope note, independently confirmed: validate.ts still has the same console.log-then-process.exit() pattern (lines 572/576) and deserves the same treatment in a follow-up.
Approving per Abhay's go-ahead.
— Abhai
3 participants
Root cause
hyperframes lint --jsonwrote the JSON payload withconsole.log()and then immediately calledprocess.exit().process.exit()terminates the process before Node flushes an asynchronously-buffered stdout — which is exactly what a non-TTY (piped) stdout is. Sohyperframes lint --json | tee,> out.json, or any CI/agent capture silently lost the entire JSON payload on Windows (reported on 0.7.31, non-TTY).The same
console.log-then-process.exitpattern was on all four exit sites (both--jsonbranches plus the human-readable and thrown-error paths), so any of them could truncate its output.Fix
Set
process.exitCodeandreturn, lettingrun()unwind so Node drains stdout before exiting with the code. This is exactly the pattern the other commands (publish/transcribe/upgrade/play/present) already use —lintwas the outlier still callingprocess.exit()right after writing output. No logic change; only the exit mechanism.Test plan
New
lint.test.tsdrives the command'srun()with mockedlintProject/resolveProjectand aprocess.exitspy that throws if called. Four cases —--jsonwith errors,--jsonclean,--jsonon a thrown error, and the human-readable errors path — each assertsprocess.exitis never called and the correctprocess.exitCodeis set. These fail against the pre-fix code (the spy throws on the firstprocess.exit).bunx vitest run packages/cli/src/commands/lint.test.ts— 4/4 passing.tsc --noEmitclean; fullbun run buildclean;oxlint/oxfmtclean.Context: this was the highest-confidence fix from a ~17.5h backlog of feedback that accumulated during a PostHog outage; several other precise reports in that batch (render viewport clipping to ~996px on macOS, webm-alpha producing opaque output on Windows, bundled SFX library shipping no mp3s, non-ASCII Windows username breaking ffmpeg paths) are logged for follow-up.